Do not evaluate shard_size and shard_min_doc_count at segment slice level #9085

jed326 · 2023-08-03T05:01:14Z

Description

Refactored InternalTerms and InternalSignificantTerms to take BucketCountThresholds in the constructor. Using this we can determine which values to use when doing reduce at the shard vs coordinator level.

This resolves the test failures in TermsShardMinDocCountIT and ShardSizeTermsIT, however it does not address the failures in TermsDocCountErrorIT.

Related Issues

Resolves #8860

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

github-actions · 2023-08-03T05:10:03Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/21753/
CommitID: 95e8eab
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

jed326 · 2023-08-03T18:42:47Z

The TermsShardMinDocCountIT ITs should pass now. There are 2 things that I'm still exploring:

In the reduce method we just use the same min_doc_count and required_size and don't need to check ReduceContext::isSliceLevel.
- This seems fine for InternalSignificantTerms, however for InternalTerms the min_doc_count evaluation actually happens after reduce so that probably won't work.
In the buildAggregations method, we can pass in different values for min_doc_count and required_size based on whether or not it's at the shard or coordinator level.
- First of all, it doesn't look like there's currently a great way to determine where buildAggregations is being called. However, even if there is then the problem with InternalTerms in [1] above makes this difficult to use.

...src/main/java/org/opensearch/search/aggregations/bucket/InternalSingleBucketAggregation.java

server/src/main/java/org/opensearch/search/aggregations/bucket/LocalBucketCountThresholds.java

reta · 2023-08-09T17:34:10Z

It looks pretty clean, thanks @jed326 !

… values Signed-off-by: Jay Deng <jayd0104@gmail.com>

opensearch-trigger-bot · 2023-08-09T17:55:41Z

Compatibility status:



> Task :checkCompatibility
Incompatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/performance-analyzer.git]
Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

BUILD SUCCESSFUL in 36m 56s

github-actions · 2023-08-09T17:56:56Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/22260/
CommitID: 2b210a6
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-08-09T18:18:37Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/22262/
CommitID: 3b55340
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

opensearch-trigger-bot · 2023-08-09T18:18:52Z

Compatibility status:



> Task :checkCompatibility
Incompatible components: [https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/performance-analyzer.git]
Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

BUILD SUCCESSFUL in 35m 33s

jed326 · 2023-08-09T18:32:30Z

https://build.ci.opensearch.org/job/gradle-check/22262/#showFailuresLink

Build failures are from unrelated cluster routing tests. @reta I think we are good to merge if it looks good to you. Thanks!

reta · 2023-08-09T18:34:19Z

Build failures are from unrelated cluster routing tests. @reta I think we are good to merge if it looks good to you. Thanks!

Yes, flaky tests but we need green checks, rerurn it

github-actions · 2023-08-09T19:06:22Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/22271/
CommitID: 3b55340

opensearch-trigger-bot · 2023-08-09T19:22:14Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-9085-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 15b7de00f2ff75587560ac6e38ffb20da030178e
# Push it to GitHub
git push --set-upstream origin backport/backport-9085-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-9085-to-2.x.

reta · 2023-08-09T19:24:52Z

@jed326 sadly backport failed, could you please send manual one?

…evel (opensearch-project#9085) * Use BucketCountThresholds in InternalTerms and InternalAggregations and do not apply shard level thresholds at slice level for Concurrent Segment Search Signed-off-by: Jay Deng <jayd0104@gmail.com> * Addressing comments Signed-off-by: Jay Deng <jayd0104@gmail.com> * Re-introduce shardSize member to InternalMultiTerms and InternalMappedTerms Signed-off-by: Jay Deng <jayd0104@gmail.com> * Introduce LocalBucketCountThresholds for local size and min_doc_count values Signed-off-by: Jay Deng <jayd0104@gmail.com> --------- Signed-off-by: Jay Deng <jayd0104@gmail.com>

…evel (#9085) (#9211) * Use BucketCountThresholds in InternalTerms and InternalAggregations and do not apply shard level thresholds at slice level for Concurrent Segment Search * Addressing comments * Re-introduce shardSize member to InternalMultiTerms and InternalMappedTerms * Introduce LocalBucketCountThresholds for local size and min_doc_count values --------- Signed-off-by: Jay Deng <jayd0104@gmail.com>

…evel (opensearch-project#9085) * Use BucketCountThresholds in InternalTerms and InternalAggregations and do not apply shard level thresholds at slice level for Concurrent Segment Search Signed-off-by: Jay Deng <jayd0104@gmail.com> * Addressing comments Signed-off-by: Jay Deng <jayd0104@gmail.com> * Re-introduce shardSize member to InternalMultiTerms and InternalMappedTerms Signed-off-by: Jay Deng <jayd0104@gmail.com> * Introduce LocalBucketCountThresholds for local size and min_doc_count values Signed-off-by: Jay Deng <jayd0104@gmail.com> --------- Signed-off-by: Jay Deng <jayd0104@gmail.com> Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

…evel (opensearch-project#9085) * Use BucketCountThresholds in InternalTerms and InternalAggregations and do not apply shard level thresholds at slice level for Concurrent Segment Search Signed-off-by: Jay Deng <jayd0104@gmail.com> * Addressing comments Signed-off-by: Jay Deng <jayd0104@gmail.com> * Re-introduce shardSize member to InternalMultiTerms and InternalMappedTerms Signed-off-by: Jay Deng <jayd0104@gmail.com> * Introduce LocalBucketCountThresholds for local size and min_doc_count values Signed-off-by: Jay Deng <jayd0104@gmail.com> --------- Signed-off-by: Jay Deng <jayd0104@gmail.com> Signed-off-by: Ivan Brusic <ivan.brusic@flocksafety.com>

…evel (opensearch-project#9085) * Use BucketCountThresholds in InternalTerms and InternalAggregations and do not apply shard level thresholds at slice level for Concurrent Segment Search Signed-off-by: Jay Deng <jayd0104@gmail.com> * Addressing comments Signed-off-by: Jay Deng <jayd0104@gmail.com> * Re-introduce shardSize member to InternalMultiTerms and InternalMappedTerms Signed-off-by: Jay Deng <jayd0104@gmail.com> * Introduce LocalBucketCountThresholds for local size and min_doc_count values Signed-off-by: Jay Deng <jayd0104@gmail.com> --------- Signed-off-by: Jay Deng <jayd0104@gmail.com> Signed-off-by: Shivansh Arora <hishiv@amazon.com>

jed326 changed the title ~~temp~~ Do not evaluate shard_size and shard_min_doc_count at segment slice level Aug 3, 2023

jed326 force-pushed the shard-params-fix branch from 95e8eab to d1e5d1c Compare August 3, 2023 18:34

jed326 force-pushed the shard-params-fix branch from d1e5d1c to 978b105 Compare August 3, 2023 18:47

jed326 marked this pull request as ready for review August 3, 2023 18:48

jed326 commented Aug 3, 2023

View reviewed changes

...src/main/java/org/opensearch/search/aggregations/bucket/InternalSingleBucketAggregation.java Outdated Show resolved Hide resolved

reta reviewed Aug 9, 2023

View reviewed changes

server/src/main/java/org/opensearch/search/aggregations/bucket/LocalBucketCountThresholds.java Outdated Show resolved Hide resolved

reta approved these changes Aug 9, 2023

View reviewed changes

Introduce LocalBucketCountThresholds for local size and min_doc_count…

3b55340

… values Signed-off-by: Jay Deng <jayd0104@gmail.com>

jed326 force-pushed the shard-params-fix branch from 2b210a6 to 3b55340 Compare August 9, 2023 17:42

reta merged commit 15b7de0 into opensearch-project:main Aug 9, 2023

reta added the backport 2.x Backport to 2.x branch label Aug 9, 2023

jed326 mentioned this pull request Aug 10, 2023

[Backport 2.x] Do not evaluate shard_size and shard_min_doc_count at segment slice level #9211

Merged

6 tasks

jed326 deleted the shard-params-fix branch September 13, 2023 19:10

lukas-vlcek mentioned this pull request Sep 19, 2023

[DOC] Terms (sub-)aggregation missing some low level parameters and accuracy details opensearch-project/documentation-website#5046

Closed

4 tasks

jed326 mentioned this pull request Dec 12, 2023

[Concurrent Segment Search] Performance regression for multi-term aggs on high cardinality data #11584

Closed

jed326 mentioned this pull request Jan 10, 2024

[Concurrent Segment Search] shard_min_doc_count is not enforced at the slice level #11847

Open

jed326 mentioned this pull request Jun 19, 2024

[AUTOCUT] Gradle Check Flaky Test Report for MinDocCountIT #14313

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not evaluate shard_size and shard_min_doc_count at segment slice level #9085

Do not evaluate shard_size and shard_min_doc_count at segment slice level #9085

jed326 commented Aug 3, 2023 •

edited

Loading

github-actions bot commented Aug 3, 2023

jed326 commented Aug 3, 2023

reta commented Aug 9, 2023

opensearch-trigger-bot bot commented Aug 9, 2023

github-actions bot commented Aug 9, 2023

github-actions bot commented Aug 9, 2023

opensearch-trigger-bot bot commented Aug 9, 2023

jed326 commented Aug 9, 2023

reta commented Aug 9, 2023

github-actions bot commented Aug 9, 2023

opensearch-trigger-bot bot commented Aug 9, 2023

reta commented Aug 9, 2023

Do not evaluate shard_size and shard_min_doc_count at segment slice level #9085

Do not evaluate shard_size and shard_min_doc_count at segment slice level #9085

Conversation

jed326 commented Aug 3, 2023 • edited Loading

Description

Related Issues

Check List

github-actions bot commented Aug 3, 2023

Gradle Check (Jenkins) Run Completed with:

jed326 commented Aug 3, 2023

reta commented Aug 9, 2023

opensearch-trigger-bot bot commented Aug 9, 2023

github-actions bot commented Aug 9, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 9, 2023

Gradle Check (Jenkins) Run Completed with:

opensearch-trigger-bot bot commented Aug 9, 2023

jed326 commented Aug 9, 2023

reta commented Aug 9, 2023

github-actions bot commented Aug 9, 2023

Gradle Check (Jenkins) Run Completed with:

opensearch-trigger-bot bot commented Aug 9, 2023

reta commented Aug 9, 2023

jed326 commented Aug 3, 2023 •

edited

Loading